Skip to content

fix(fmt): JSX <pre> with comments now formats stably#34556

Open
divybot wants to merge 2 commits into
mainfrom
orch/divybot-300
Open

fix(fmt): JSX <pre> with comments now formats stably#34556
divybot wants to merge 2 commits into
mainfrom
orch/divybot-300

Conversation

@divybot

@divybot divybot commented May 30, 2026 •
edited by littledivy
Loading

Copy link
Copy Markdown
Contributor

Summary

deno fmt panics with Formatting not stable. Bailed after 5 tries when a <pre> JSX element contains a {/* … */} expression-container comment. The minimal repro from #20403 is:

import { ErrorPageProps } from "\$fresh/server.ts";

export default function ErrorPage500(props: ErrorPageProps) {
  return (
    <pre>
      {props.url}
      {/* {props.url} */}
    </pre>
  );
}

Each formatting pass duplicates the comment outside the JSX expression container:

    <pre>
      {props.url}
      {/* {props.url} */}
    /* {props.url} */          // <- new copy added every pass
    </pre>

so the formatter never reaches a stable output and the embedding harness bails out.

Cause

<pre> element bodies in dprint-plugin-typescript are emitted verbatim as raw text instead of going through gen_jsx_with_opening_and_closing. That bypass skips the bookkeeping that marks comments inside the body as handled, so when gen_node is later called for the closing </pre> tag, the {/* … */} comment is picked up again by leading_comments_with_previous and emitted a second time as a leading comment of the closing tag.

Fix

The fix is in dprint-plugin-typescript (dprint/dprint-plugin-typescript#796): in the <pre> branch of gen_jsx_element, advance the comment tracker past the body range and mark every comment it visited as handled. The closing tag's leading-comment pass then finds nothing left to emit and the format stabilises on the first pass.

This PR consumes the upstream fix via a [patch.crates-io] git entry until it ships in a new crates.io release; the patch can be replaced with a plain version bump once that release is out.

Test plan

  • tests/specs/fmt/jsx_pre_with_comment/__test__.jsonc uses the exact input from Deno fmt fails (Preact component with comment) #20403 and asserts the formatter produces the same text back.
  • The upstream PR adds 2 dprint spec cases covering this and an adjacent-comments variant; the existing 649 dprint specs continue to pass.

Closes #20403
Closes denoland/divybot#300

bartlomieju
bartlomieju requested changes Jun 1, 2026
View reviewed changes

@bartlomieju bartlomieju left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't merge it until the dprint-plugin-typescript is released

@bartlomieju bartlomieju added the upstream Changes in upstream are required to solve these issues label Jun 7, 2026
@littledivy littledivy force-pushed the orch/divybot-300 branch 3 times, most recently from d2eb0bd to 9916c5e Compare June 15, 2026 22:14
@divybot @littledivy
fix(fmt): JSX <pre> with comments now formats stably
69fe554 
`deno fmt` panics with `Formatting not stable. Bailed after 5 tries` when a
`<pre>` JSX element contains a `{/* … */}` expression-container comment, e.g.

```tsx
function ErrorPage500(props: ErrorPageProps) {
  return (
    <pre>
      {props.url}
      {/* {props.url} */}
    </pre>
  );
}
```

`<pre>` element bodies in `dprint-plugin-typescript` are emitted verbatim as
raw text instead of going through the normal JSX child generation path. That
bypass skips the bookkeeping that marks comments inside the body as handled,
so when `gen_node` is later called for `</pre>` the comment is picked up
again as a leading comment of the closing tag and emitted as a JS block
comment outside the expression container. Each formatting pass adds one
more copy of the comment, so the formatter never stabilises.

The fix is in `dprint-plugin-typescript`
(dprint/dprint-plugin-typescript#796) and is consumed here via a
`[patch.crates-io]` git entry until the upstream change ships in a new
crates.io release. A spec regression test in
`tests/specs/fmt/jsx_pre_with_comment` uses the exact input from the issue
and asserts the formatter produces the same text back.

Closes #20403
Closes denoland/orchid#300

Co-Authored-By: Divy Srivastava <me@littledivy.com>
@divybot @littledivy
test(fmt): move jsx_pre_with_comment expected output into expected.out
d795829 
`tests/specs/mod.rs` panics if an inline `output` string exceeds 160
characters, so the spec test for the JSX `<pre>` comment regression
failed on every platform with "The 'output' property in your
__test__.jsonc file is too long." Extract the expected text into
`expected.out` and reference it from `__test__.jsonc`.

Co-Authored-By: Divy Srivastava <me@littledivy.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bartlomieju bartlomieju bartlomieju requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

upstream Changes in upstream are required to solve these issues

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Deno fmt fails (Preact component with comment)

2 participants

@divybot @bartlomieju